-
Notifications
You must be signed in to change notification settings - Fork 2.2k
InfluxDB: Update for InfluxDB 3, condense, add metadata #2600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bb33542
to
5f5128c
Compare
- Updated Docker documentation for InfluxDB 3 Core and Enterprise - Docker Compose examples - Support channel preferences - Shorten version docs to startup essentials and link to full documentation - Categories and other metadata Co-authored-by: meelahme <[email protected]>
5f5128c
to
f9b7c2b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
@sanderson @bnpfeife Please review this PR (I can't seem to actually request reviews yet).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content looks good, but I'm not authorized to approve it.
@jstirnaman Hey, this repository is part of the Docker organization (not InfluxData). I also don't have permissions to review this. So, we'll need to wait for someone from Docker to review. We'll probably just need to be patient considering the sheer volume of changes that the Docker folk must see on a daily basis. However, the changes do look good to me. I did notice that the file permissions were changed from 0644 (read) to 755 (read + execute). I am unsure, but I suspect this is a remnant from editing the files on Windows. If a reviewer requires it, I can help get these back to 644. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few notes -- I don't love how much Enterprise/licensed content we've got here (especially when it's just "here's how to do it on the OSS version, and here's all the same content again but with the tiny addition for the Enterprise edition", and because Enterprise editions are already a special case in DOI that will likely not be permitted to persist much longer; https://github.com/docker-library/official-images#what-are-official-images).
Also, it looks like we need to make more use of the %%IMAGE%%
template variable, and I would indeed like to see the unnecessary/incorrect executable bits on the files dropped as discussed upthread.
- Replace v1 Enterprise image aliases with version-specific tags - Changed all instances of %%IMAGE%%:meta to %%IMAGE%%:1.11-meta - Changed all instances of %%IMAGE%%:data to %%IMAGE%%:1.11-data - Focus InfluxDB 3 section primarily on Core with brief Enterprise mentions - Ensure consistent use of %%IMAGE%% template variable throughout
influxdb/logo.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed one in the executability fixes 👀
Moved to draft: I'm updating the README further to better address version issues and work with influxdata/influxdata-docker#820 |
1. Updated content.md with the new structure and major version tags. 2. Use `1.11` tags until `1.12` release or new `1` major version tags are published. 3. Fixed compose.yaml to use port 8181 directly 4. Updated variant files to use %%IMAGE%%:1-data and %%IMAGE%%:1-meta (these don't seem to be used) 5. Removed references to latest tag from v2 description 6. Created GitHub issue docker-library#820 in influxdata/influxdata-docker requesting to create generic v1 tags
a013177
to
708fe42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but should be reviewed by more knowledgeable people.
Details:
1.11
tags until1.12
release or new1
major version tags are published.Closes influxdata/docs-v2#6019
Updates and replaces PR
Coauthor: @MeelahMe